Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable validation-webhook daemonset for managed clusters #361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smalleni
Copy link
Collaborator

@smalleni smalleni commented Apr 1, 2022

When running the node density workloads on managed clusters, starting
with 4.10.5 on ROSA, we are seeing an error like "admission webhook
"regular-user-validation.managed.openshift.io" denied the request" when trying add a label to a
node directly by using oc label node. The recommended way to add labels
to nodes on managed ROSA clusters is by editing the machinepool.
However, the Default machinepool cannot be edited to add labels and we
see an error like "Labels cannot be updated on the Default machine pool".
The only way to add a label is by disabling the
validation-webhook daemonset and thereby admission cotnrol in the
openshift-validation-webhook project that only exists on managed services
clusters. We disable the daemonset by adding a fake nodeSelector before
labeling the nodes and remove thenodeSelector after unlabeling the nodes.
Adding a nodeSelector on top of the existing nodeAffinity means that both
the conditions needs to be met for a pod to be scheduled. Also by adding
the nodeSelector, the spec is not overwritten during reconcillation whereas
changes to nodeAffinity are being overwritten.

Signed-off-by: Sai Sindhur Malleni [email protected]

Description

Fixes

@smalleni smalleni force-pushed the managed_validation_webhook branch from 3d25801 to abae979 Compare April 1, 2022 02:13
@smalleni smalleni requested review from dry923, morenod and mukrishn April 1, 2022 02:17
@smalleni smalleni force-pushed the managed_validation_webhook branch 3 times, most recently from c6da1e9 to c6dc386 Compare April 1, 2022 02:21
@smalleni smalleni force-pushed the managed_validation_webhook branch 3 times, most recently from 49d5f6d to e18cd8f Compare May 15, 2023 18:41
@smalleni smalleni force-pushed the managed_validation_webhook branch from e18cd8f to 8bfb31f Compare May 16, 2023 20:27
When running workloads on managed clusters with cluster-admin user, we are seeing an error like "admission webhook
"regular-user-validation.managed.openshift.io" denied the request" when trying add a label to a
node directly by using oc label node. The recommended way to add labels
to nodes on managed ROSA clusters is by editing the machinepool.
However, the Default machinepool cannot be edited to add labels and we
see an error like "Labels cannot be updated on the Default machine pool".
The only way to add a label is by disabling the
validation-webhook daemonset and thereby admission control in the
openshift-validation-webhook project that only exists on managed services
clusters. We disable the daemonset by adding a fake nodeSelector before
labeling the nodes and remove the nodeSelector after unlabeling the nodes.
Adding a nodeSelector on top of the existing nodeAffinity means that both
the conditions needs to be met for a pod to be scheduled. Also by adding
the nodeSelector, the spec is not overwritten during reconcillation whereas
changes to nodeAffinity are being overwritten.

This change4 is important for managed clusters as we don't always have access to kubeconfig (when
running in prow for example).

Signed-off-by: Sai Sindhur Malleni <[email protected]>
@smalleni smalleni force-pushed the managed_validation_webhook branch from 8bfb31f to 5280bc7 Compare May 18, 2023 02:23
@rsevilla87
Copy link
Member

rsevilla87 commented May 23, 2023

When running the node density workloads on managed clusters, starting with 4.10.5 on ROSA, we are seeing an error like "admission webhook "regular-user-validation.managed.openshift.io" denied the request" when trying add a label to a node directly by using oc label node. The recommended way to add labels to nodes on managed ROSA clusters is by editing the machinepool. However, the Default machinepool cannot be edited to add labels and we see an error like "Labels cannot be updated on the Default machine pool". The only way to add a label is by disabling the validation-webhook daemonset and thereby admission cotnrol in the openshift-validation-webhook project that only exists on managed services clusters. We disable the daemonset by adding a fake nodeSelector before labeling the nodes and remove thenodeSelector after unlabeling the nodes. Adding a nodeSelector on top of the existing nodeAffinity means that both the conditions needs to be met for a pod to be scheduled. Also by adding the nodeSelector, the spec is not overwritten during reconcillation whereas changes to nodeAffinity are being overwritten.

Signed-off-by: Sai Sindhur Malleni [email protected]

Description

Fixes

curious, why do we need to label nodes?

I assume this is used for the old node-density implementation available in e2e-benchmarking, the implementation based on the kube-burner's OCP wrapper doesn't need to label nodes anymore.

Rather than keep fixing and updating the old implementation we should encourage users to use the new implementation

download_binary(){
KUBE_BURNER_URL=https://github.com/cloud-bulldozer/kube-burner/releases/download/v${KUBE_BURNER_VERSION}/kube-burner-${KUBE_BURNER_VERSION}-Linux-x86_64.tar.gz
curl -sS -L ${KUBE_BURNER_URL} | tar -xzC ${KUBE_DIR}/ kube-burner
}

check_managed_cluster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes on this file are not required as I stated in my comment

@smalleni
Copy link
Collaborator Author

When running the node density workloads on managed clusters, starting with 4.10.5 on ROSA, we are seeing an error like "admission webhook "regular-user-validation.managed.openshift.io" denied the request" when trying add a label to a node directly by using oc label node. The recommended way to add labels to nodes on managed ROSA clusters is by editing the machinepool. However, the Default machinepool cannot be edited to add labels and we see an error like "Labels cannot be updated on the Default machine pool". The only way to add a label is by disabling the validation-webhook daemonset and thereby admission cotnrol in the openshift-validation-webhook project that only exists on managed services clusters. We disable the daemonset by adding a fake nodeSelector before labeling the nodes and remove thenodeSelector after unlabeling the nodes. Adding a nodeSelector on top of the existing nodeAffinity means that both the conditions needs to be met for a pod to be scheduled. Also by adding the nodeSelector, the spec is not overwritten during reconcillation whereas changes to nodeAffinity are being overwritten.
Signed-off-by: Sai Sindhur Malleni [email protected]

Description

Fixes

curious, why do we need to label nodes?

I assume this is used for the old node-density implementation available in e2e-benchmarking, the implementation based on the kube-burner's OCP wrapper doesn't need to label nodes anymore.

Rather than keep fixing and updating the old implementation we should encourage users to use the new implementation

Oh, I didn't realize that was the case, thanks for surfacing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants